Skip to content

refactor(application-header): convert inlineDropdown to a signal#2179

Open
spike-rabbit wants to merge 1 commit into
mainfrom
refactor/application-header-inline-dropdown-signal
Open

refactor(application-header): convert inlineDropdown to a signal#2179
spike-rabbit wants to merge 1 commit into
mainfrom
refactor/application-header-inline-dropdown-signal

Conversation

@spike-rabbit

@spike-rabbit spike-rabbit commented Jun 18, 2026

Copy link
Copy Markdown
Member

Replace the inlineDropdown Observable with a signal across the application-dropdown system.
This allows easier usage, especially also in host attributes which we will need to enhance a11y in a future PR.


Documentation.
Examples.
Dashboards Demo.
Playwright report.

Coverage Reports:

Code Coverage

@spike-rabbit spike-rabbit force-pushed the refactor/application-header-inline-dropdown-signal branch from bc309a8 to cbe6230 Compare June 18, 2026 12:03

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the application header, collapsible actions, and dropdown trigger components to use Angular Signals instead of RxJS Observables for the inlineDropdown state. The review feedback highlights that using effect() to propagate state changes violates Rule 38 of the Repository Style Guide, which can cause ExpressionChangedAfterItHasBeenChecked errors and performance issues. It is recommended to replace these continuous effects with on-demand RxJS subscriptions using toObservable when the menus or dropdowns are actually open, along with cleaning up unused imports and adding the necessary RxJS operators.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread projects/element-ng/application-header/si-header-collapsible-actions.component.ts Outdated
@spike-rabbit spike-rabbit force-pushed the refactor/application-header-inline-dropdown-signal branch 2 times, most recently from 7ed3cf5 to e44910f Compare June 18, 2026 12:27
@spike-rabbit

Copy link
Copy Markdown
Member Author

the gemini feedback is entirely pointless

Replace the `inlineDropdown` Observable with a signal across the
application-header.
This allows easier usage, especially in host attributes
which we need to enhance a11y in a future PR.
@spike-rabbit spike-rabbit force-pushed the refactor/application-header-inline-dropdown-signal branch from e44910f to 638621e Compare June 18, 2026 13:10
@spike-rabbit spike-rabbit marked this pull request as ready for review June 18, 2026 15:08
@spike-rabbit spike-rabbit requested review from a team as code owners June 18, 2026 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant